Canfar Python Libraries #702

martinkilbinger · 2026-02-02T10:19:00Z

Summary

This PR updates job hanling on canfar to the new canfar python library system.

This comes with job submission and monitoring classes and scripts.

New tiles (P9) were added to the CFIS tile list.

Closes some old issues #669 .

Reviewer Checklist

Reviewers should tick the following boxes before approving and merging the PR.

…etsts

pattern

…ript

cailmdaley

Nice work modernising the CANFAR submission path and replacing CDSClient with astroquery. The documentation rewrite is a real improvement. A few things to fix before merge — mostly runtime bugs that will crash in production, plus some smaller items.

Bugs that will crash

canfar_monitor.py:130 — RunTimeError → RuntimeError
canfar_monitor.py:188 — self._get_kind() → self.get_kind() (no underscore)
canfar_monitor.py:121 — self._params["verbose"] referenced but never defined in params_default
canfar_monitor.py:206-208 — else after try/except runs on success, not failure. The "Failed to destroy" message fires when destruction succeeds.
canfar_monitor.py:222,235 — "failed{estr}" missing f prefix
summary.py:218,235 — mgs instead of msg — NameError at runtime
clear_ngmix_prev.py:117 — globs ngmix_out_dir when it should glob prev_out_dir
distribute_tiles.py:124 — compares string args with int literals (0, 1) — will never match, should be ('0', '1')

Intentional?

make_cat.py:348 — threshold changed from 0.9 to 0.0. Since a ratio can't be negative, this disables the check entirely. If intentional, the dead code block could be removed or at least commented with why.

Style / design

distribute_tiles.py:1 — hardcoded shebang #!/arc/home/kilbinger/...
uncompress_fits.py:62 — bare except: — worth catching something specific
uncompress_fits.py:75-76 — the TFORM fix runs outside the if name: guard, so on empty keys value/comment are stale from the previous iteration
canfar_submit.py:32-33 — the != 512 guard can never trigger (set to 512 on the line above)
canfar_submit.py:260 — AsyncSession() opened twice (once in run_async, again in _submit_single_batch)
pyproject.toml — requires-python downgraded from >=3.11 to >=3.10 without mention. Also canfar, sf_tools, h5py, pandas added as core deps — could these live under an optional [canfar] extra?
Star imports in summary_run.py and summary_params_pre_v2.py

Typos

canfar_monitor.py:3 — "Montiot" → "Monitor"
canfar_monitor.py:247 — "Retreiving" → "Retrieving"
mask.py:498 — "astroquer" → "astroquery"
canfar_submit.py:116 — "SESSSION" (triple S)
mask.py:496-501 — IndexError raised with positional args instead of a formatted string

Tests

None of the new code has tests. The mgs/msg typo in summary.py would have been caught immediately.

src/shapepipe/canfar/canfar_monitor.py

src/shapepipe/modules/mask_package/mask.py

scripts/python/clear_ngmix_prev.py

scripts/python/distribute_tiles.py

src/shapepipe/utilities/summary.py

martinkilbinger · 2026-02-06T06:47:43Z

Check again please @cailmdaley, I answered to all points of your PR report. Thanks!

- canfar_monitor: "Montior" typo, try/except/else flow for bulk destroy - canfar_submit: "debut_out" dict key typo - uncompress_fits: header assignment inside if-name guard - distribute_tiles: check arg value not flag name for dry_run Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

cailmdaley · 2026-02-06T15:26:49Z

Hey Martin, thanks for the fixes! I took the liberty of pushing a few more small ones I spotted while verifying:

canfar_monitor.py — "Montior" → "Monitor" (the old typo became a new one), and the bulk destroy try/except still printed "Success" unconditionally (added an else: clause)
canfar_submit.py — dict key debut_out → debug_out
uncompress_fits.py — header[name] = (value, comment) was still outside the if name: guard
distribute_tiles.py — dry_run check was comparing the flag name (-n) against ("0", "1") instead of the flag's value

Let me know if I made any mistakes. Have a nice weekend!

sfarrens

Hey @martinkilbinger sorry for the delay. I am a bit rusty on ShapePipe 😅, so I may have missed something, but overall the proposed changes look fine. I opened a few threads to resolve some minor issues.

environment.yml

sfarrens · 2026-02-09T14:41:31Z

src/shapepipe/summary_run.py

 from shapepipe.utilities.summary import *

-from summary_params_pre_v2 import *
+from shapepipe.utilities.summary_params_pre_v2 import *


Not really important for this PR, but in general it is not good practice to use import * as it makes it harder to trace the origin of objects. It would be better to from shapepipe.utilities import summary_params_pre_v2 if you need the whole module or to simple import the objects you actually need.

src/shapepipe/summary_run.py

sfarrens · 2026-02-09T15:02:51Z

src/shapepipe/canfar_run.py

+    Main program
+    """
+    # Scripts to call canfar classes are created by pyproject.toml
+    return 0


What is the point of this function?

sfarrens · 2026-02-09T15:04:03Z

src/shapepipe/canfar_run.py

This is fine, but strictly speaking there should be some docstrings to explain the point of the module and the corresponding functions.

sfarrens · 2026-02-09T15:18:10Z

src/shapepipe/canfar/canfar_submit.py

+        self._patch = os.environ["patch"] if "patch" in os.environ else "P0"
+
+        # Set job parameters
+        version = "1.1"


Why is this hardcoded here?

sfarrens · 2026-02-09T15:21:26Z

src/shapepipe/canfar/canfar_submit.py

+                print(f"Each session will process ~{math.ceil(total_n / num_replicas)} jobs using chunk()")
+                print("Sessions = ", sessions)
+            except Exception as e:
+                print(f"❌ CANFAR session.create() failed: {type(e).__name__}: {e}")


I would be surprised if printing emojis worked universally, but fine if it works where you need to run it.

sfarrens · 2026-02-09T15:22:50Z

src/shapepipe/canfar/canfar_submit.py

I didn't spot any major issues, but more docstrings would be good.

sfarrens · 2026-02-09T15:24:06Z

src/shapepipe/canfar/canfar_monitor.py

+    def check_params(self):
+        pass


What is the point of this?

sfarrens · 2026-02-09T15:25:36Z

src/shapepipe/canfar/canfar_monitor.py

Same point re: docstrings

martinkilbinger added 30 commits October 31, 2025 09:27

python version 3.10 synchronised

cd34cb4

New tiles for patch "P9"

c39aca4

init run script: fixed usage string

2ecb54c

update to tiles_DR6 dir

a35f55f

patch P9 exp processing, canfar queue

cff89be

using fitsio for new tile weights (astropy fits TFORM=1PE(625) bug"

acaa63e

added astroquery

c2a95a7

cleane up, new Vizier python guide star commands

8d9254c

fixed bug for test mode

01ec68c

canfar scripts installing

aa244f6

added canfar classes

f9741a5

testing bulk submission

28c222f

canfar submission jobs updated

97562a6

pyprojkect

70e7bfd

Merge branch 'p9' of github.com:martinkilbinger/shapepipe-1 into p9

a5f9cfd

canfar submission debug

2e472f9

Merge branch 'p9' of github.com:martinkilbinger/shapepipe-1 into p9

6c352ce

canfar submit -P -J to set parallel jobs

af2343c

4096 back to n_mult=4, working

3b631d4

Merge branch 'p9' of github.com:martinkilbinger/shapepipe-1 into p9

4ae479a

distribute tiles: debug output

66ce6e8

canfar submit script: added parallel jobs, jobs per session; remove t…

27a016a

…etsts

summary: fixed (?) split exp runner search

90be433

pattern

added canfar_log_monitor script; fixed REPLICA COUNT bug in submit sc…

533ee55

…ript

cleand up output

ac4b281

Merge branch 'p9' of github.com:martinkilbinger/shapepipe-1 into p9

f60a8ae

improved canfar log monitor script

f224a32

init run script: set -m 1 -s 1 as default

a9ea963

Merge branch 'p9' of github.com:martinkilbinger/shapepipe-1 into p9

e5c1974

improved monitoring log; improved debug output

d4b7b8c

martinkilbinger added 10 commits January 8, 2026 07:10

cleaned up init_run script

b9e7b7a

added quiet flag to canfar_monitor

fa92bdd

make_cat_runner: remove error when SEx and ngmix obj lengths are diff

f3e2e1e

ran black

cf6e7ab

added h5py to pyproject

65bba5e

doc post-processing

8cb7ae5

Merge branch 'CosmoStat:develop' into p9

08d5ec3

pipeline post-processing doc

5b2f57b

updated pipepine doc with merged star cat

cb76b34

Merge remote-tracking branch 'upstream/develop' into p9

110cd2b

martinkilbinger changed the title P9 Canfar Python Libraries Feb 2, 2026

martinkilbinger mentioned this pull request Feb 2, 2026

[BUG] canfar minor issues #669

Open

martinkilbinger linked an issue Feb 2, 2026 that may be closed by this pull request

[BUG] canfar minor issues #669

Open

This was referenced Feb 4, 2026

V1.3.x #703

Draft

Coverage mask #699

Open

martinkilbinger requested review from cailmdaley, fabianhervaspeters and sfarrens February 4, 2026 08:19

cailmdaley reviewed Feb 5, 2026

View reviewed changes

changed answering PR report

ebbf8bf

martinkilbinger removed request for fabianhervaspeters and sfarrens February 6, 2026 06:47

martinkilbinger and others added 2 commits February 6, 2026 07:48

Merge remote-tracking branch 'upstream/develop' into p9

f5341d2

Updated required python version to 3.12 (cosmo-numbe req)

c8c44e3

martinkilbinger marked this pull request as ready for review February 7, 2026 09:10

sfarrens reviewed Feb 9, 2026

View reviewed changes

Changes according to PR report; doc strings, clean-up

6dc4c2b

Canfar Python Libraries #702

Are you sure you want to change the base?

Canfar Python Libraries #702

Uh oh!

Conversation

martinkilbinger commented Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Reviewer Checklist

Uh oh!

cailmdaley left a comment

Choose a reason for hiding this comment

Bugs that will crash

Intentional?

Style / design

Typos

Tests

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

martinkilbinger commented Feb 6, 2026

Uh oh!

cailmdaley commented Feb 6, 2026

Uh oh!

sfarrens left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

martinkilbinger commented Feb 2, 2026 •

edited

Loading